Fix health-check close failing auth in vMCP BackendClient#4613
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes vMCP backend session close requests failing authentication during health checks by ensuring the backend HTTP transport consistently re-injects the originating request’s identity (when present) and the health-check context marker onto all outgoing requests—including the synthetic DELETE request generated by mcp-go’s StreamableHTTP.Close().
Changes:
- Extend
identityPropagatingRoundTripperto propagate both identity and the health-check marker on every request. - Capture
isHealthCheckat client/transport creation time and re-apply it duringRoundTrip, coveringClose()’s context-less DELETE. - Add focused unit tests verifying identity propagation, health-check marker propagation, and propagation of both together.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/vmcp/client/client.go | Propagates health-check marker (in addition to identity) via transport to prevent auth failures on Close() DELETE. |
| pkg/vmcp/client/client_test.go | Adds unit tests validating identity + health-check marker propagation behavior in the transport. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4613 +/- ##
=======================================
Coverage 68.60% 68.60%
=======================================
Files 516 517 +1
Lines 53841 53850 +9
=======================================
+ Hits 36937 36946 +9
+ Misses 14057 14056 -1
- Partials 2847 2848 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mcp-go's StreamableHTTP.Close() creates a DELETE request with context.Background(), discarding the health-check marker and identity from the original call context. The identityPropagatingRoundTripper only stored identity, so when ListCapabilities deferred c.Close(), the DELETE hit auth strategies with neither a health-check marker nor an identity, producing "no identity found in context" errors. Fix by capturing isHealthCheck at transport creation time and re-injecting both identity and the health-check marker into every outgoing request, including the synthetic DELETE from Close(). Fixes #4573
jerm-dro
left a comment
There was a problem hiding this comment.
Clean fix, good test coverage. One suggestion for a regression acceptance test inline.
Summary
mcp-go's StreamableHTTP.Close() creates a DELETE request with context.Background(), discarding the health-check marker and identity from the original call context. The identityPropagatingRoundTripper only stored identity, so when ListCapabilities deferred c.Close(), the DELETE hit auth strategies with neither a health-check marker nor an identity, producing "no identity found in context" errors.
Fix by capturing isHealthCheck at transport creation time and re-injecting both identity and the health-check marker into every outgoing request, including the synthetic DELETE from Close().
Fixes #4573
Type of change
Test plan
task test)task test-e2e)task lint-fix)Changes
Does this introduce a user-facing change?
Special notes for reviewers